Added Assignment updates/ variables#93
Conversation
Added in variables for if a channel is assigned to a mix, group, or fx.
| this.emit('pan_level', makeNRPN(msb, lsb), vc, vf) | ||
| } | ||
| // Assign NRPNs | ||
| else if (0x60 <= msb && msb <= 0x6f) { |
There was a problem hiding this comment.
Make the upper bound 0x6e here, please -- the protocol docs go from 60 00 to 6E 6E, and we ought not pass along info for NRPNs outside the actual range. (Tho if distant memory of this code serves, nothing is actually going to go wrong, exactly, for nonexistent NRPNs here.)
| * For each source-sink relationship with assign parameters, invoke the given | ||
| * function. | ||
| */ | ||
| export function forEachSourceSinkAssign(model: Model, f: SourceSinkAssignFunctor): void { |
There was a problem hiding this comment.
This is sensible-ish enough and all, but it's also avoidable copying. 🙂 I landed a rev generalizing the original function -- please rebase this atop that and make use of the new functionality.
...or wait, see later comments, I don't think you'll need to do this.
| forEachSourceSinkLevel(model, getLevel) | ||
| forEachOutputLevel(model, getLevel) | ||
|
|
||
| // Request assign NRPNs as well so assignment variables are populated at startup |
There was a problem hiding this comment.
I dislike the system of variables that only exist sometimes, and others you will into existence as setExtraVariable(s) (see elsewhere in module code). But the unfortunate reality is that gobs and gobs of variables are what the module has today, and some are Less Important than others and only come to life at later times.
I think this is better served by making these extra variables, that are only added/created as assign NRPN messages are received -- similar to what's done for pan/balance changes. My guess is that dynamic changes to assignments are closer to "as unlikely as pan/balance changes" than to "as likely as level or muting changes", and therefore defining these as extra variables is more sensible than defining them as variables that are always present.
...which, ugh, I guess means forEachSourceSink* didn't actually immediately need to be made generic, did it, and I devoted effort to something not actually important. Oh well.
| verboseLog(`Assign received: ${prettyNRPN(nrpn)}, VC=${prettyByte(vc)}, VF=${prettyByte(vf)}`) | ||
|
|
||
| const { MSB, LSB } = splitNRPN(nrpn) | ||
| const variableId = `assign_${MSB}.${LSB}` |
There was a problem hiding this comment.
At some point these variables -- or some other storage mechanism that makes use of variables' new (as of the 2.0 module API) ability to store any JSON value -- need to not directly incorporate NRPN or MSB/LSB, but instead should be like level_inputChannel1_group5 or something. I hesitate to create more mess like this to eventually require cleanup.
But the hard reality is that redoing all the variable naming/structuring will break every user anyway when it happens. And I don't yet have a way to work backwards from NRPN to the particular relationship it refers to. So I guess just doubling down on the existing approach is acceptable.
| const { MSB, LSB } = splitNRPN(nrpn) | ||
| const variableId = `assign_${MSB}.${LSB}` | ||
| // Use '1'/'0' string values for Companion variables | ||
| instance.setVariableValues({ [variableId]: vf !== 0 ? '1' : '0' }) |
There was a problem hiding this comment.
Assignment being exposed as true or false seems preferable to me. Why do you think we ought expose them as strings containing number 1 or 0 instead? Boolean values are more amenable to use in expressions, which I intend at some point to convert the module to using everywhere.
| }) | ||
| const commands = sinks.map((sink) => this.#assignToSink(source, active, sink, calc)) | ||
|
|
||
| // XXX |
There was a problem hiding this comment.
Please leave this as-is -- it's alluding to how the promise returned here is dropped on the floor and if it rejects, that rejection goes entirely unhandled, a problem I haven't really figured out how to properly solve yet.
| : this.#assignToSink(source, active, mixOrLR, mixNrpnCalc) | ||
| }) | ||
|
|
||
| // XXX |
There was a problem hiding this comment.
Please leave this as-is, as noted in the subsequent comment.
| const lrNrpnCalc = AssignNRPNCalculator.get(this.model, [sourceType, 'lr']) | ||
|
|
||
| const commands = mixes.map((mixOrLR) => { | ||
| const commands: readonly number[][] = mixes.map((mixOrLR) => { |
There was a problem hiding this comment.
If you're changing this at all (it shouldn't be necessary to change anything here), make it NRPNDataMessage[]
2 participants
Added in variables for if a channels or mixes or whatever is assigned to a mix, group, or fx.